Skip to content

RU-T50 Build fix#242

Merged
ucswift merged 1 commit into
masterfrom
develop
May 13, 2026
Merged

RU-T50 Build fix#242
ucswift merged 1 commit into
masterfrom
develop

Conversation

@ucswift
Copy link
Copy Markdown
Member

@ucswift ucswift commented May 13, 2026

Summary by CodeRabbit

  • Bug Fixes
    • Enhanced iOS build compatibility by applying a configuration fix across all native dependencies instead of a single target. This addresses framework module compilation issues and improves compatibility with modern Xcode versions.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 13, 2026

📝 Walkthrough

Walkthrough

This PR updates the Expo config plugin to apply the Xcode 26+ non-modular header include fix to all pod targets rather than a single target. The documentation is updated to reflect the broader fix scope, and the injected Podfile logic changes from targeting livekit_react_native_webrtc specifically to iterating all installer.pods_project targets.

Changes

WebRTC Framework Compatibility Fix

Layer / File(s) Summary
Expand CLANG setting to all pod targets
plugins/withWebRTCFrameworkFix.js
Documentation updated to describe the Xcode 26+ non-modular include fix for all pods. The Ruby loop now iterates all installer.pods_project targets and sets CLANG_ALLOW_NON_MODULAR_INCLUDES_IN_FRAMEWORK_MODULES = 'YES' for each target's build configurations, replacing the previous single-target approach.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

  • Resgrid/Unit#241: Introduces the same plugins/withWebRTCFrameworkFix.js plugin; this PR expands its target scope from a single pod to all pods.

Poem

🐰 A patch so small, yet wise and true,
All pods now shimmer in Xcode's view,
One target grew to many more,
Modular checks are now no chore! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'RU-T50 Build fix' is vague and does not clearly describe the main change in the pull request. Provide a more descriptive title that explains the specific nature of the build fix, such as 'Fix Xcode 26+ non-modular includes in WebRTC framework for all pod targets' or 'Apply CLANG_ALLOW_NON_MODULAR_INCLUDES setting to all pods'.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch develop

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
plugins/withWebRTCFrameworkFix.js (1)

24-27: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Marker check can skip the new all-target fix on already-patched Podfiles.

If the Podfile already contains this marker from a previous single-target patch, the plugin returns early and never applies the new all-target loop, so the upgrade path can remain broken.

Suggested fix
-      const marker = 'CLANG_ALLOW_NON_MODULAR_INCLUDES_IN_FRAMEWORK_MODULES';
-      if (contents.includes(marker)) {
+      const marker = "installer.pods_project.targets.each do |target|";
+      if (contents.includes(marker)) {
         return config;
       }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@plugins/withWebRTCFrameworkFix.js` around lines 24 - 27, The current
early-return checks only for the marker
'CLANG_ALLOW_NON_MODULAR_INCLUDES_IN_FRAMEWORK_MODULES' in contents and returns
config, which skips adding the new all-target loop when a Podfile was previously
patched for a single target; update the logic in withWebRTCFrameworkFix.js so
that instead of returning immediately when contents.includes(marker) is true you
also verify the presence of the new all-target loop (e.g., the loop text you
intend to inject) and only return if both the marker and the all-target loop
exist; if the marker exists but the all-target loop is missing, continue to
apply the new patch to inject the all-target loop before returning config.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@plugins/withWebRTCFrameworkFix.js`:
- Around line 24-27: The current early-return checks only for the marker
'CLANG_ALLOW_NON_MODULAR_INCLUDES_IN_FRAMEWORK_MODULES' in contents and returns
config, which skips adding the new all-target loop when a Podfile was previously
patched for a single target; update the logic in withWebRTCFrameworkFix.js so
that instead of returning immediately when contents.includes(marker) is true you
also verify the presence of the new all-target loop (e.g., the loop text you
intend to inject) and only return if both the marker and the all-target loop
exist; if the marker exists but the all-target loop is missing, continue to
apply the new patch to inject the all-target loop before returning config.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f7fe367f-0229-4fef-abc2-51191fa333f1

📥 Commits

Reviewing files that changed from the base of the PR and between 02b6516 and 24c6ea3.

📒 Files selected for processing (1)
  • plugins/withWebRTCFrameworkFix.js

@ucswift
Copy link
Copy Markdown
Member Author

ucswift commented May 13, 2026

Approve

Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR is approved.

@ucswift ucswift merged commit c534ed6 into master May 13, 2026
19 of 20 checks passed
This was referenced May 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant